Parquet Variant Extract Benchmark - #23616
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughChangesVARIANT benchmark
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cpp/benchmarks/io/parquet/experimental/variant/extract.cpp`:
- Around line 19-22: Add the direct declaration headers <cstdint>,
<cuda_runtime_api.h>, and <tuple> to the includes in extract.cpp for its
fixed-width integer types, cudaStreamSynchronize, and std::ignore usage.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: c1a8c46b-67f1-4a01-bbe5-91ef1b75588e
📒 Files selected for processing (2)
cpp/benchmarks/CMakeLists.txtcpp/benchmarks/io/parquet/experimental/variant/extract.cpp
| #include <cstring> | ||
| #include <memory> | ||
| #include <string> | ||
| #include <vector> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="cpp/benchmarks/io/parquet/experimental/variant/extract.cpp"
for header in cuda_runtime_api.h cstdint tuple; do
rg -qF "`#include` <${header}>" "$file" ||
printf 'Missing direct include: <%s>\n' "$header"
doneRepository: rapidsai/cudf
Length of output: 262
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="cpp/benchmarks/io/parquet/experimental/variant/extract.cpp"
printf '%s\n' '--- include block and symbol context ---'
sed -n '1,45p' "$file"
sed -n '205,240p' "$file"
printf '%s\n' '--- direct declarations and usages ---'
rg -n 'cudaStreamSynchronize|std::ignore|int(8|16|32|64)_t|uint(8|16|32|64)_t|size_t|ptrdiff_t|make_tuple|tie|tuple' "$file"Repository: rapidsai/cudf
Length of output: 5883
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- project usage of the required direct headers ---'
rg -n -g '*.{cpp,cu,h,hpp,cuh}' '`#include` <cuda_runtime_api\.h>|cudaStreamSynchronize|`#include` <cstdint>|`#include` <tuple>' cpp/benchmarks/io/parquet/experimental cpp/benchmarks | head -200Repository: rapidsai/cudf
Length of output: 841
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- CUDA runtime include conventions ---'
rg -n -g '*.{cpp,cu,h,hpp,cuh}' '`#include` <cuda_runtime(_api)?\.h>|cudaStreamSynchronize' cpp | head -200Repository: rapidsai/cudf
Length of output: 3365
Add direct declaration headers.
This file uses fixed-width integer types, cudaStreamSynchronize, and std::ignore without their declaration headers. Add <cstdint>, <cuda_runtime_api.h>, and <tuple>.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cpp/benchmarks/io/parquet/experimental/variant/extract.cpp` around lines 19 -
22, Add the direct declaration headers <cstdint>, <cuda_runtime_api.h>, and
<tuple> to the includes in extract.cpp for its fixed-width integer types,
cudaStreamSynchronize, and std::ignore usage.
Source: Coding guidelines
Description
This PR adds a microbenchmark (VARIANT_NVBENCH) exercising cudf::io::parquet::experimental::extract_variant_field and cast_variant. It measures performance across these axes:
num_rows: 32 K / 256 K / 2 M rowstype: int32_t, float, bool, string, arraynesting_level– 0 (primitive), 1 (single-key object), 5 (deeply nested)hit_rate– 20% or 80% rows containing a correctly typed value at the target pathThis solves one of the PRs in story issue #22312.
Checklist